Have arrays' `drop_glue` just unsize and call the slice version by scottmcm · Pull Request #155184 · rust-lang/rust

scottmcm · 2026-04-12T05:42:21Z

It's silly to emit two loops (because of the drop ladder -- just one in panic=abort) for every array length that's dropped when we can just polymorphize to the slice version.

Built atop #154327 to avoid conflicts later, so draft for now.

r? @WaffleLapkin

rustbot · 2026-04-12T05:42:28Z

WaffleLapkin is not on the review rotation at the moment.
They may take a while to respond.

scottmcm · 2026-04-12T05:47:45Z

+
+// RAW: ; core::ptr::drop_glue::<[array_drop_glue::NeedsDrop; [[N:7|13|42]]]>
+// RAW-NEXT: inlinehint
+// RAW: call core::ptr::drop_glue::<[array_drop_glue::NeedsDrop]>


View changes since the review

Compare nightly https://rust.godbolt.org/z/5Wv1q86ja with a loop in each of the three.

WaffleLapkin · 2026-04-14T10:40:31Z

💭 does it make sense to keep the array case separate for N <= 1 or something?

scottmcm · 2026-04-15T01:20:10Z

My understanding is that LLVM will very effectively inline those -- just like how we don't special case the array iterator for N ≤ 1 and still use the polymorphic version.

Does make me ponder if we should have a separate general "hey, there's no Drop so we can delegate to the one for the one field" kind of check, but I'd leave all these for a different PR if that's ok.

rustbot · 2026-05-09T08:08:14Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

scottmcm · 2026-05-09T08:11:05Z

@bors try @rust-timer queue

Have arrays' `drop_glue` just unsize and call the slice version

rust-bors · 2026-05-09T10:21:17Z

☀️ Try build successful (CI)
Build commit: ec33dfc (ec33dfc30f643cd46f52d91e53fe4fe66cd8d655, parent: 4a997eeefbb8cf41617324bb6d08c72171c614e2)

rust-timer · 2026-05-09T11:14:00Z

Finished benchmarking commit (ec33dfc): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.3%	[-0.3%, -0.3%]	1

Max RSS (memory usage)

Results (primary -5.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-5.2%	[-5.2%, -5.2%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-5.2%	[-5.2%, -5.2%]	1

Cycles

Results (secondary -2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.8%	[-5.4%, -3.8%]	3
All ❌✅ (primary)	-	-	0

Binary size

Results (primary 0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.2%, 1.1%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[-0.5%, 1.1%]	4

Bootstrap: 497.359s -> 500.627s (0.66%)
Artifact size: 397.13 MiB -> 397.30 MiB (0.04%)

WaffleLapkin

I have a few minor questions, but the change looks good to me, so r=me, unless you think it would be valuable to add comments to code/test.

View changes since this review

WaffleLapkin · 2026-05-11T11:21:13Z

+        start.terminator = Some(Terminator {
+            source_info,
+            kind: TerminatorKind::Call {
+                func: Operand::function_handle(tcx, def_id, [ty::GenericArg::from(slice_ty)], span),


(question) What's the def_id a def id of?

It's the def id of the drop_glue itself. Basically the same as require_lang_item(LangItem::DropGlue), but it was already passed in to generate the drop glue we're currently making, so I didn't need to look it up -- the existing parameter on https://doc.rust-lang.org/nightly/nightly-rustc/rustc_mir_transform/shim/fn.build_drop_shim.html.

WaffleLapkin · 2026-05-11T11:24:50Z

+// fragility from that changing we don't check any particular order.
+
+// RAW: ; core::ptr::drop_glue::<[array_drop_glue::NeedsDrop; [[N:7|13|42]]]>
+// RAW-NOT: inlinehint


(question) Why are you checking for lack of inlinehint here?

Oh, because I'd not un-pushed the second commit where I experimented with not touching the inlining check.

I went back to the original which I think got overall-better perf results where the delegating ones are inlinehint but the one with the actual loop(s) isn't, and thus these tests check that.

WaffleLapkin · 2026-05-11T11:29:21Z

+
+// CHECK-LABEL: ; core::ptr::drop_glue::<[array_drop_glue::NeedsDrop]>
+// CHECK-NOT: inlinehint
+// OPT: add nuw nsw {{.+}} 1


What is this checking for?

That it's the actual loop which got optimized to add nuw nsw despite just being Add in the code generating the loop.

scottmcm · 2026-05-11T18:02:50Z

@bors r=WaffleLapkin

rust-bors · 2026-05-11T18:02:54Z

📌 Commit 05f52c7 has been approved by WaffleLapkin

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

JonathanBrouwer · 2026-05-11T19:36:17Z

@bors rollup
Perf seems to be noise

@WaffleLapkin

…, r=WaffleLapkin Have arrays' `drop_glue` just unsize and call the slice version It's silly to emit two loops (because of the drop ladder -- just one in panic=abort) for every array length that's dropped when we can just polymorphize to the slice version. Built atop rust-lang#154327 to avoid conflicts later, so draft for now. r? @WaffleLapkin

…uwer Rollup of 11 pull requests Successful merges: - #156448 (miri subtree update) - #155023 (Introduce move expressions (`move($expr)`) ) - #156429 (Simplify `intrinsic::raw_eq` in MIR when possible) - #147672 (LLBC-linker: Do not strip debug symbols for the nvptx target anymore) - #155169 (jsondoclint: simplify code using idiomatic Rust) - #155184 ( Have arrays' `drop_glue` just unsize and call the slice version) - #156022 (rustdoc: Fix cosmetic issues when reporting unresolved paths in `broken_intra_doc_links`) - #156442 (Show intrinsics::gpu in docs) - #156461 (LLVM 23: Specify `returnaddress` intrinsic return type) - #156462 (LLVM 23: Accept float (instead of hex) literals in codegen tests) - #156466 (Refactor `CheckAttrVisitor` so rustfmt can format it.)

rustbot assigned WaffleLapkin Apr 12, 2026

scottmcm commented Apr 12, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

scottmcm mentioned this pull request Apr 12, 2026

change the type of the argument of drop_in_place lang item to &mut _ #154327

Merged

scottmcm force-pushed the intercept-array-drop-shim branch from d1bdee8 to 2ae092a Compare April 15, 2026 01:23

rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` PG-exploit-mitigations Project group: Exploit mitigations labels Apr 15, 2026

scottmcm removed PG-exploit-mitigations Project group: Exploit mitigations A-run-make Area: port run-make Makefiles to rmake.rs A-test-infra-minicore Area: `minicore` test auxiliary and `//@ add-core-stubs` labels Apr 15, 2026

This comment has been minimized.

Sign in to view

scottmcm added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Apr 28, 2026

scottmcm force-pushed the intercept-array-drop-shim branch 2 times, most recently from 8a17db9 to bf354cd Compare May 7, 2026 03:22

rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label May 7, 2026

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels May 7, 2026

Have arrays' drop_glue just unsize and call the slice version

05f52c7

scottmcm force-pushed the intercept-array-drop-shim branch from 1f28062 to b0020c9 Compare May 9, 2026 08:08

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 9, 2026

rust-bors Bot pushed a commit that referenced this pull request May 9, 2026

Auto merge of #155184 - scottmcm:intercept-array-drop-shim, r=<try>

ec33dfc

Have arrays' `drop_glue` just unsize and call the slice version

This comment has been minimized.

Sign in to view

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels May 9, 2026

WaffleLapkin approved these changes May 11, 2026

View reviewed changes

WaffleLapkin added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2026

scottmcm force-pushed the intercept-array-drop-shim branch from b0020c9 to 05f52c7 Compare May 11, 2026 16:36

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 11, 2026

This was referenced May 11, 2026

Rollup of 8 pull requests #156470

Closed

Rollup of 11 pull requests #156473

Merged

rust-bors Bot merged commit aeffd15 into rust-lang:main May 12, 2026
11 checks passed

rustbot added this to the 1.97.0 milestone May 12, 2026

Uh oh!

Conversation

scottmcm commented Apr 12, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 12, 2026

Uh oh!

scottmcm Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

WaffleLapkin commented Apr 14, 2026

Uh oh!

scottmcm commented Apr 15, 2026

Uh oh!

This comment has been minimized.

rustbot commented May 9, 2026

Uh oh!

scottmcm commented May 9, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented May 9, 2026

Uh oh!

This comment has been minimized.

rust-timer commented May 9, 2026

Overall result: ✅ improvements - no action needed

Uh oh!

WaffleLapkin left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottmcm commented May 11, 2026

Uh oh!

rust-bors Bot commented May 11, 2026

Uh oh!

JonathanBrouwer commented May 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

scottmcm commented Apr 12, 2026 •

edited by rustbot

Loading

scottmcm Apr 12, 2026 •

edited

Loading

WaffleLapkin left a comment •

edited by rustbot

Loading